Skip to content

cli: Fix datafusion-cli hint edge cases#20609

Merged
goldmedal merged 1 commit intoapache:mainfrom
comphead:spark
Mar 2, 2026
Merged

cli: Fix datafusion-cli hint edge cases#20609
goldmedal merged 1 commit intoapache:mainfrom
comphead:spark

Conversation

@comphead
Copy link
Contributor

@comphead comphead commented Feb 27, 2026

Which issue does this PR close?

  • Closes #.

Rationale for this change

The hint show condition relied on just checking if the input string is empty which caused visual issues like

image

What changes are included in this PR?

  helper.rs:                                                
  - Added a show_hint: Cell<bool> field to CliHelper. Uses Cell for interior mutability since hint() takes &self.                                                                                          
  - hint() now sets show_hint to false the moment line is non-empty. The hint only displays when show_hint is still true and the line is empty — i.e., only on the fresh prompt before any key is pressed. 
  - validate() calls reset_hint() after validation so the hint reappears on the next prompt after submission.                                                                                              
  - Added pub fn reset_hint(&self) for external reset.                                                                                                                                                     
                                                            
  exec.rs:
  - On Ctrl+C (ReadlineError::Interrupted), calls rl.helper().unwrap().reset_hint() so the next prompt still shows the hint.

Are these changes tested?

Are there any user-facing changes?

return true;
}
false
DFParser::parse_sql(&sql).is_ok_and(|stmts| {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its just simplification

@comphead comphead changed the title cli: Fix cli hint edge cases cli: Fix datafusion-cli hint edge cases Feb 28, 2026
Copy link
Contributor

@goldmedal goldmedal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @comphead. Look great 👍

@goldmedal goldmedal added this pull request to the merge queue Mar 2, 2026
Merged via the queue into apache:main with commit 43584ca Mar 2, 2026
30 checks passed
@comphead
Copy link
Contributor Author

comphead commented Mar 2, 2026

Thanks folks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants